-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LLDB: Extend the dump_page methods #4420
Conversation
try: | ||
flidx = "%3d" % freelist.index(obj_addr) | ||
except ValueError: | ||
flidx = ' ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like objects that are not T_NONE
will get an empty { }
in the output for flidx
. I think this is some confusing output for objects that are not T_NONE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that was the intended behaviour. It allows you to easily visualise which slots are part of the freelist and which ones aren't. Like this:
bits: [ ] T_NONE [401]: { 2} Addr: 0x1007ebed0 (flags: 0x0)
bits: [ ] T_NONE [402]: { 1} Addr: 0x1007ebef8 (flags: 0x0)
bits: [ ] T_NONE [403]: { 0} Addr: 0x1007ebf20 (flags: 0x0)
bits: [LM ] T_CLASS [404]: { } Addr: 0x1007ebf48 (flags: 0x100001062)
bits: [LM ] T_PAYLOAD [405]: { } Addr: 0x1007ebf70 (flags: 0x3077)
It's easy to see where slots have been assigned values and where the freelist pointer (and list) is going. Although I suspect we could make it more obvious in the output that { }
was the freelist index? How about:
bits: [ ] T_NONE idx: [401] freelist_idx: { 2} Addr: 0x1007ebed0 (flags: 0x0)
bits: [ ] T_NONE idx: [402] freelist_idx: { 1} Addr: 0x1007ebef8 (flags: 0x0)
bits: [ ] T_NONE idx: [403] freelist_idx: { 0} Addr: 0x1007ebf20 (flags: 0x0)
bits: [LM ] T_CLASS idx: [404] freelist_idx: { } Addr: 0x1007ebf48 (flags: 0x100001062)
bits: [LM ] T_PAYLOAD idx: [405] freelist_idx: { } Addr: 0x1007ebf70 (flags: 0x3077)
the empty { }
mirrors the behaviour of rp
- it shows an empty [ ]
section when no bits are set
bits: [ ]
T_NONE: (RBasic) *$35 = (flags = 0x0000000000000000, klass = 0x00000001007ebea8)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like your second one 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I'll patch that and push it up.
misc/lldb_cruby.py
Outdated
dump_bits(target, result, page, obj_addr, end= " ") | ||
flags = obj.GetChildMemberWithName('flags').GetValueAsUnsigned() | ||
flType = flags & RUBY_T_MASK | ||
|
||
flidx = ' ' | ||
if flType == RUBY_T_NONE: | ||
try: | ||
flidx = "%3d" % freelist.index(obj_addr) | ||
except ValueError: | ||
flidx = ' ' | ||
|
||
result_str = "%s [%3d]: {%s} Addr: %0#x (flags: %0#x)" % (rb_type(flags, ruby_type_map), page_index, flidx, obj_addr, flags) | ||
|
||
if highlight == obj_addr: | ||
result_str = ' '.join([result_str, "<<<<<"]) | ||
|
||
print(result_str, file=result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if this shared logic with rp
, that way the output would be more consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean so that we display the same output here and for rp
? If so then I think this is a different use case - we only ever want to display a single dense line of info here so that we can see all of the slots on a page in a compact arrangement.
rp
does a lot more stuff to display the body of the structs, almost all of which we're not using in this context.
rather than having to do this in a two step process: 1. heap_page obj 2. dump_page $2 (or whatever lldb variable heap_page set) we can now just dump_page_rvalue obj
fdd6932
to
5d2e409
Compare
I frequently use
dump_page
in lldb to visualise heap pages, but the process of seeing the heap page containing aVALUE
is annoying. First we have toheap_page obj
to get the page and then calldump_page
on the returned page address.This PR adds
dump_page_rvalue
that combines both steap, dumping the page containing theVALUE
passed as an argument - and also highlights that slots position in the page.